Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary MonadIO and MonadFail uses #177

Merged
merged 2 commits into from
Oct 1, 2019

Conversation

glguy
Copy link
Collaborator

@glguy glguy commented Sep 30, 2019

MonadIO usage in this library was causing unnecessary liftIO calls to be littered across the package. The package doesn't need any of this generality, and anyone wanting to use MonadIO outside of the package can use liftIO as needed. It might make sense to rely on MonadIO if there were other type classes in use as well, but without that the generalization is premature and make this more complex than needed to perform the task. This also cleans up some of the MonadFail needs that were coming up because of some uses of fail leaking outside of the other liftIO wrappers.

@glguy glguy requested a review from jtdaugherty September 30, 2019 23:53
@jtdaugherty jtdaugherty merged commit 6aa1d05 into jtdaugherty:master Oct 1, 2019
@jtdaugherty
Copy link
Owner

Thank you!

@jtdaugherty
Copy link
Owner

anyone wanting to use MonadIO outside of the package can use liftIO as needed

FWIW, I don't think this addresses my comment elsewhere; they can use liftIO to embed calls to e.g. Output fields in their own code, but if they were wanting for some reason to embed their own monadic values in the Output fields, liftIO doesn't help with that. Granted, given that the m isn't part of the Output type, I don't see what possible value anyone could gain by embedding their own m values in the Output fields, so I doubt it's going to cause a lot of trouble.

@glguy glguy deleted the just-io-patch branch October 1, 2019 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants